Skip to content

[HLSL] Create HLSL legalization passes #123811

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

s-perron
Copy link
Contributor

The semantics of HLSL require passes some passes to be run that will
modify the llvm-ir created by clang to match what is expected by the
backend.

In this PR, wewe move the DXILFinalizeLinkage to be run for all backend.
The pass is also renamed to HLSLFinalizeLinkage. A pass of the global opt
pass is run afterwards.

Fixes #122767.

@@ -61,7 +61,7 @@ unsigned RemoveDupes(unsigned Buf[MAX], unsigned size) {
}


RWBuffer<unsigned> Indices;
RWBuffer<unsigned> Indices : register(u0);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resources without explicit register assignments are not supported yet. If we do not do this then we end up with a handle that is zero initialized, which causes problems.

Copy link

github-actions bot commented Jan 21, 2025

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 6518b121f037717fd211c36659f7b25266424719 834f6be33005c6fd289e776ad5dbeeebc5509695 --extensions cpp,h -- clang/lib/CodeGen/BackendUtil.cpp llvm/lib/Target/DirectX/DirectXTargetMachine.cpp llvm/include/llvm/Transforms/HLSL/HLSLFinalizeLinkage.h llvm/lib/Transforms/HLSL/HLSLFinalizeLinkage.cpp
View the diff from clang-format here.
diff --git a/llvm/lib/Transforms/HLSL/HLSLFinalizeLinkage.cpp b/llvm/lib/Transforms/HLSL/HLSLFinalizeLinkage.cpp
index 8b0b6ea1c1..143349b6c5 100644
--- a/llvm/lib/Transforms/HLSL/HLSLFinalizeLinkage.cpp
+++ b/llvm/lib/Transforms/HLSL/HLSLFinalizeLinkage.cpp
@@ -18,7 +18,7 @@ using namespace llvm;
 static bool finalizeLinkage(Module &M) {
   SmallPtrSet<Function *, 8> Funcs;
 
-  for(auto &Var : M.globals()) {
+  for (auto &Var : M.globals()) {
     if (Var.getLinkage() == GlobalValue::ExternalLinkage) {
       Var.setLinkage(GlobalValue::InternalLinkage);
     }

The semantics of HLSL require passes some passes to be run that will
modify the llvm-ir created by clang to match what is expected by the
backend.

In this PR, wewe move the DXILFinalizeLinkage to be run for all backend.
The pass is also renamed to HLSLFinalizeLinkage. A pass of the global opt
pass is run afterwards.

Fixes llvm#122767.
@s-perron
Copy link
Contributor Author

@llvm-beanz Let me know if this is a reasonable direction to go. I'll have update a lot of tests, and I don't want to do that unless we believe this is what we would like to do.

@bogner
Copy link
Contributor

bogner commented Feb 3, 2025

Sharing the code here makes sense to me, however, this does make the pass run significantly earlier in the DXIL pipeline. That might be fine but we'll have to make sure we aren't creating dead functions in any of the other DXIL backend passes that might have been cleaned up by this so far.

@s-perron s-perron closed this Mar 6, 2025
@s-perron s-perron deleted the handle_wrapper branch March 6, 2025 15:38
@s-perron s-perron restored the handle_wrapper branch April 3, 2025 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[HLSL] RWBuffer resource variable has external linkage
2 participants